SOCKS5 Fix - #486
Conversation
9b03594 to
84e6786
Compare
…SOCKS5 handshake, and lower default to 1s
|
Hi @erebe — following up on #485 and this PR. Rebased onto current main, tests pass. Are you interested in this fix/PR? If not no-worries I'll go ahead and close #486 and #485. Let me know! The bug still reproduces on v10.6.2. The CONNECT path in socks5/tcp_server.rs calls reply_success with a hardcoded 127.0.0.1:0 before any upstream connection is attempted, so the client is told it succeeded whether or not the target is reachable. This is distinct from #515, which bounds the handshake but doesn't change when the reply is sent. One design note: #515 moved this path to Socks5ServerProtocol, which owns the socket and only hands it back from reply_success — so the reply is now withheld through a small adapter instead of taking the socket early. Happy to do it another way if you'd prefer. If the objection is scope rather than the bug, the first two commits are the actual fix; the timeout work can be dropped. |
|
Hello, Thank you for the PR. I am interested in it and fixing those issues. I will try to look at it during summer 🙈 I don't plan further changes in wstunnel (beside webtransport addition that I just added), so it is safe on your side, you will not get more merge conflict I guess |
Summary
This PR fixes SOCKS5
CONNECTreply timing for both forward and reverse tunnel paths. Reported as an issue in #485.Before this change, SOCKS5 success could be returned before the upstream target connect result was known. In practice, proxy tooling (
proxychains+nc/nmap -sT) could observe false-positive connect success for unreachable ports.After this change, SOCKS5 success/failure is sent only when the real upstream connect outcome is known.
Problem
In affected paths, the SOCKS5 listener acknowledged
CONNECTtoo early.This breaks expected SOCKS5 semantics and can produce incorrect scan/probe results.
Root Cause
REP=Succeededimmediately after SOCKS handshake acceptance, instead of waiting for upstream tunnel connect outcome.What Changed
--timeout-connect(alias--timeout-connect-sec)WSTUNNEL_TIMEOUT_CONNECT3sBehavioral Outcomes
proxychains-based TCP connect probing now reflects target state more accurately.Validation
Commands run:
cargo fmt --all -- --checkcargo checkcargo check -p wstunnelcargo test -p wstunnel test_socks5_tunnel_connect_success --libcargo test -p wstunnel test_socks5_tunnel_connect_failure --libcargo test -p wstunnel reverse_socks5_handshake --libAll passed.
Notes
fast_socks5::server::Socks5Server, jwt validation deprecation, unfulfilled lint expectation).--timeout-connect(alias--timeout-connect-sec) andWSTUNNEL_TIMEOUT_CONNECT.nmap/proxychains.Reviewer-Oriented Change Summary (File-by-File)
wstunnel/src/protocols/socks5/tcp_server.rsSocks5WriteHalf::Tcpto holdpending_replystate.CONNECTsuccess reply in accept loop.send_reply_if_needed()to emit a one-shot SOCKS reply only when connect outcome is known.AsyncWriteimpl for new enum shape.wstunnel/src/tunnel/client/client.rsconnect()result to server transport.HANDSHAKE_CONNECT_OK/FAIL) to report local connector outcome back to server.wstunnel/src/tunnel/server/server.rshandle_tunnel_request()now carries whether request is reverse-socks5 and uses a writer type that can be downcast safely.self.config.timeout_connectin connector paths.wstunnel/src/tunnel/server/handler_websocket.rswstunnel/src/tunnel/server/handler_http2.rswstunnel/src/tunnel/transport/websocket.rsread_handshake_byte()andprefetchedbuffer inWebsocketTunnelRead.wstunnel/src/tunnel/transport/http2.rsread_handshake_byte()andprefetchedbuffer inHttp2TunnelRead.wstunnel/src/tunnel/reverse_socks5.rs(new)wstunnel/src/tunnel/server/socks5_reply.rs(new)AnyAsyncWritetrait and helper to downcast writer toSocks5WriteHalfsafely and send reply if needed.wstunnel/src/tunnel/server/mod.rssocks5_replymodule exports.wstunnel/src/tunnel/mod.rsreverse_socks5module internally.wstunnel/src/test_integrations.rsCONNECTsuccess and failure over wstunnel.wstunnel/src/config.rs--timeout-connect(alias--timeout-connect-sec) andWSTUNNEL_TIMEOUT_CONNECTto both client and server options.3s.wstunnel/src/lib.rstimeout_connectdefaults with CLI-configured values for both client and server runtime config.Disclosure
Code changes, repro script, issue draft, and this PR draft were prepared with assistance from ChatGPT 5.3 Codex.
I manually reviewed the code and manually tested the behavior before submitting this PR.